ci: build python wheels for more python versions and x86 macos architectures#1183
ci: build python wheels for more python versions and x86 macos architectures#1183
Conversation
|
There is no change in the changelog. This PR will not produce a new releasable version. |
Taepper
force-pushed
the
1174-more-python-platforms
branch
from
4a82ac0 to
b8694bc
Compare
Taepper
force-pushed
the
1174-more-python-platforms
branch
from
b8694bc to
6f6ccd9
Compare
There was a problem hiding this comment.
Pull request overview
This PR enhances the Python bindings build process to support multiple Python versions (3.11-3.14) and adds x86_64 macOS architecture support, improving compatibility across different platforms and Python installations.
Changes:
- Added support for building wheels for Python 3.11, 3.12, 3.13, and 3.14
- Implemented matrix strategy for building macOS wheels on both arm64 and x86_64 architectures
- Simplified setup.py by reusing existing CMake build directories and removing redundant conan path configuration
- Updated CMakeLists.txt to use Development.Module for better compatibility with minimal Python installations
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pyproject.toml | Added Python 3.13 and 3.14 version classifiers |
| setup.py | Refactored CMake configuration to reuse build directories and explicitly set Python include paths for python-build-standalone compatibility |
| CMakeLists.txt | Changed from Development to Development.Module for more specific Python component requirements |
| Makefile | Renamed build-wheel to build-wheels and added loop to build for multiple Python versions (3.11-3.14) |
| .github/workflows/ci.yml | Added matrix strategy for macOS builds (arm64 and x86_64) and updated Linux builds to create wheels for multiple Python versions |
Comments suppressed due to low confidence (1)
setup.py:99
- The explicit check for conan dependencies has been removed. The old code would provide a helpful error message if conan dependencies were not found, guiding users on how to install them. The new code will fail with a less informative CMake error if the dependencies are missing. Consider adding a check to verify that the generators directory exists before running cmake, or at least update any user-facing documentation to clearly explain the prerequisite of running 'make dependencies' or 'python3 ./build_with_conan.py --release' before building wheels.
# Reuse the existing cmake build directory (populated by `make dependencies`)
# so that silolib is not recompiled for each Python version
build_dir = pjoin(source, "build", config_name)
if not os.path.isdir(build_dir):
self.mkpath(build_dir)
# Change to the build_dir directory
with changed_dir(build_dir):
# --- CONFIGURE ---
cmake_options = [
f'-DCMAKE_INSTALL_PREFIX={install_prefix}',
f'-DPython3_EXECUTABLE={sys.executable}',
# Explicitly provide include dir so CMake finds headers for
# python-build-standalone installs (used by uv) on Linux
f'-DPython3_INCLUDE_DIR={sysconfig.get_path("include")}',
f'-DCMAKE_BUILD_TYPE={config_name}',
'-DBUILD_PYTHON_BINDINGS=ON',
# var ignored on other OSs
'-DCMAKE_OSX_DEPLOYMENT_TARGET=15.0',
]
print(f"-- Running cmake to configure project in {build_dir}")
print(f"-- Will install to: {install_prefix}")
self.spawn(['cmake'] + cmake_options + [source])
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # var ignored on other OSs | ||
| '-DCMAKE_OSX_DEPLOYMENT_TARGET=15.0', |
There was a problem hiding this comment.
Setting CMAKE_OSX_DEPLOYMENT_TARGET to 15.0 will make the wheels only compatible with macOS 15 Sequoia and later. This is quite restrictive and excludes users on macOS 14, 13, and earlier versions. Consider lowering this to a more widely compatible version like 11.0 or 12.0 to maximize compatibility across different macOS versions, especially for x86_64 builds which typically support older hardware. The comment says the var is "ignored on other OSs" but doesn't justify why 15.0 is required for macOS.
| # var ignored on other OSs | |
| '-DCMAKE_OSX_DEPLOYMENT_TARGET=15.0', | |
| # var ignored on other OSs; 12.0 chosen to maximize macOS compatibility | |
| '-DCMAKE_OSX_DEPLOYMENT_TARGET=12.0', |
| # Reuse the existing cmake build directory (populated by `make dependencies`) | ||
| # so that silolib is not recompiled for each Python version | ||
| build_dir = pjoin(source, "build", config_name) | ||
|
|
||
| if not os.path.isdir(build_dir): | ||
| self.mkpath(build_dir) | ||
|
|
||
| # Change to the build_dir directory | ||
| with changed_dir(build_dir): | ||
| # --- CONFIGURE --- | ||
| cmake_options = [ | ||
| f'-DCMAKE_INSTALL_PREFIX={install_prefix}', | ||
| f'-DPython3_EXECUTABLE={sys.executable}', | ||
| # Explicitly provide include dir so CMake finds headers for | ||
| # python-build-standalone installs (used by uv) on Linux | ||
| f'-DPython3_INCLUDE_DIR={sysconfig.get_path("include")}', | ||
| f'-DCMAKE_BUILD_TYPE={config_name}', | ||
| f'-DCMAKE_PREFIX_PATH={conan_generators_dir}', | ||
| f'-DCMAKE_MODULE_PATH={conan_generators_dir}', | ||
| '-DBUILD_PYTHON_BINDINGS=ON', | ||
| # var ignored on other OSs | ||
| '-DCMAKE_OSX_DEPLOYMENT_TARGET=15.0', | ||
| ] | ||
|
|
||
| print(f"-- Running cmake to configure project in {build_temp}") | ||
| print(f"-- Running cmake to configure project in {build_dir}") | ||
| print(f"-- Will install to: {install_prefix}") | ||
| self.spawn(['cmake'] + cmake_options + [source]) |
There was a problem hiding this comment.
When building wheels for multiple Python versions sequentially, each invocation of setup.py will run cmake configure in the same build/Release directory with different Python3_EXECUTABLE and Python3_INCLUDE_DIR values. This could lead to CMake cache conflicts where the second run might use cached values from the first run. Consider adding a mechanism to either clear the Python-specific CMake cache entries or use separate build directories for each Python version (e.g., build/Release-py311, build/Release-py312) while still reusing the compiled silolib.
resolves #1174
Summary
This improves compatibility of the python bindings
PR Checklist
- [ ] All necessary documentation has been adapted or there is an issue to do so.- [ ] The implemented feature is covered by an appropriate test.